fix(server): name the process holding the port instead of a raw ServeError - #7703
fix(server): name the process holding the port instead of a raw ServeError#7703DraftProducts wants to merge 1 commit into
Conversation
…Error When the HTTP server could not bind, startup failed with a bare `ServeError` whose only detail was a stack frame in @effect/platform-node. A user whose background service already held 127.0.0.1:3773 saw the desktop backend die every 20s with nothing naming the port or the culprit. EADDRINUSE now fails with a `PortInUseError` that names the port and host, and names the holding pid when server-runtime.json describes that same port. Without that corroboration it names only the port and points at `t3 service status` rather than inventing a culprit. The original ServeError is kept as the cause, and every other bind failure passes through untouched. Model: claude-opus-5. Harness: Claude Code.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This PR improves error messaging when port binding fails, providing actionable guidance (including which process may hold the port) instead of raw EADDRINUSE errors. Changes are self-contained to error paths with comprehensive test coverage and no impact to normal server operation. You can add or adjust custom eligibility rules. Learn more. |
What Changed
A bind failure carrying
EADDRINUSEnow fails asPortInUseError, which namesthe port and, when
server-runtime.jsoncorroborates it, the process holding it.Every other bind failure passes through untouched, and the original
ServeErrorand errno error stay attached as
cause, so nothing is lost for debugging.Corroborated:
When the state file is missing or names a different port, no culprit is invented:
holderPidis set only when the persisted state parses and its port matches theone being bound. That is the same shape — and the same hedge about stale
descriptors and reused pids — that
migrate-dev-db.tsalready uses for adatabase held open by a running server.
explainPortInUsewrapsrunServerrather thanHttpServerLive. Wrapping thelayer is the more precise spot, but
Layer.catchCausetypes the recovery asLayer<ROut & ROut2>and a failing fallback layer hasROut2 = never, whichcollapses the
HttpServeroutput tonever.Layer.launchis the first pointwhere the failure is an ordinary Effect error again, and
ServerConfigisalready in its requirement channel, so no new requirement reaches
bin.ts. Thepredicate walks the
causechain forcode === "EADDRINUSE"across bothFailand
Die, since@effect/platform-nodefails the layer build with aServeErrorwhile
BunHttpServerthrows the errno error as a defect. Catching atrunServeris marginally broader than the bind alone; the only other server built during
startup is in the CLI auth flow, and the EADDRINUSE predicate keeps an unrelated
failure from being relabeled.
Tests: five cases in
portInUse.test.ts, three of which build a realNodeHttpServer.layeragainst a port held by a livenet.Server, so the failureunder test is a genuine EADDRINUSE rather than a synthetic cause. They assert the
full corroborated message, the full degraded message, that a state file naming
another port does not produce a pid, that a Bun-shaped defect is still explained,
and that a
ServeErrorwrappingEACCEScomes through identical. StubbingexplainPortInUseto identity fails four of the five.server.test.ts(124) andbin.test.ts(17) pass, as doapps/servertypecheck, lint and format. Nopre-existing failures on this path.
Why
ServeErrornames nothing. A user running the background service in WSL, withthe desktop app launching its own backend into the same distro on the same port,
got only this — in a log file they had to know to look for:
The server knows which port it was told to bind, and
server-runtime.jsonisalready read by
t3 triageandt3 pairto find a live server, so the answer to"who has it" is one file read away at exactly the moment it matters.
The message names the service explicitly because that is the common way to end up
with a T3 Code server you forgot about, but it has to stay correct for a plain
npx t3on a busy port, which is why the degraded variant claims nothing itcannot support.
Not addressed: the pid is not liveness-checked.
triage.tsandmigrate-dev-db.tsbothprocess.kill(pid, 0)before claiming a server runs, sothere is precedent, but a second heuristic with its own false positives did not
earn its place next to a message that already hedges. Separately,
cli/config.ts:262-265returnsDEFAULT_PORTwith no availability check indesktopmode wherewebmode callsfindAvailablePort— that is why thereported user hit this on every relaunch, and it is a different fix.
Checklist
Changes by Claude Opus 5 running in Claude Code.
Note
Low Risk
Error-path only: remaps EADDRINUSE after bind fails and does not change listen, auth, or persistence. Holder pid is not liveness-checked, which the message already hedges.
Overview
When the HTTP server fails to bind with
EADDRINUSE, startup now fails asPortInUseErrorinstead of a rawServeError. The message names the port and, ifserver-runtime.jsonlists the same port, the recorded pid—plus how to stop the background service or drop a stale descriptor. Other bind failures are unchanged.explainPortInUsewrapsrunServerafterLayer.launchso NodeServeErrorand Bun defects both get rewritten without collapsing theHttpServerlayer type. Tests occupy a real loopback port and cover corroborated pid, missing/mismatched state, Bun-shaped defects, and non-EADDRINUSE errors.Reviewed by Cursor Bugbot for commit 5d78054. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace raw
ServeErrorwithPortInUseErrornaming the process holding the portPortInUseErrorin portInUse.ts, carrying host, port, optionalholderPid, and a human-readable message with remediation steps when a pid is known.explainPortInUse, an effect transformer that detectsEADDRINUSEthrough up to three nested causes, readsserver-runtime.json, and fails withPortInUseErrorincluding the persisted pid when the port matches.Layer.launch(makeServerLayer)in server.ts withexplainPortInUseso server startup surfaces the holder pid instead of a raw platform error.EACCES) pass through unchanged; the pid is only included whenserver-runtime.jsonexists and refers to the same port.Macroscope summarized 5d78054.